Improve handling of dart format stderr output #148
Conversation
Previously the `stdout` and `stderr` from the format execution were merged and there was special handling for 1 specific warning message followed by a standard 2 lines of boilerplate. Now `stdout` an `stderr` are read separately. `stderr` is ignored unless there is no output on `stdout`. This filters any warnings that didn't prevent formatting, including the old hardcoded warning as well as a new one around resolving `analysis_options.yaml` and any future warnings that may come up. When there is no `stdout` content treat the `stderr` as using the diagnostic format. Ignore any lines that don't match the error format. I'm not currently aware of any output here that we _wouldn't_ want to hide, but if error output becomes important for the user to see we may need to revisit this design. - Migrate from `systemlist` to `job_start` which allows separating the output streams. - Pull the result handling out into a separate function so it can be the callback for when the output has been read.
|
I started getting bad formatting results because we only handle 1 hardcoded output on stderr before the formatting results and I now commonly see cc @munificent - I think the formatter will always fail to resolve out-of-package includes right? Should we be suppressing this in the formatter output? This change is very broad in what it ignores on stderr - should we consider still only ignoring hardcoded messages we expect to be safe to hide, and expose the rest as error messages? WDYT @sigmundch |
I'm not sure what you mean by "out-of-package" includes. If the nearest If it's annoying, we could add a flag to suppress it. |
|
I like your approach of ignoring stderr if there is a valid output in stdout. Do we have a guarantee that the output will be empty if any issue arises? Or do we need a secondary signal, like For the error modality, I worry we may hide an actionable warning/error, but I don't want to spam users with useless messages. Rather than always showing potentially non-actionable warning messages that don't match the diagnostic template, what if we only show extra warnings if we couldn't match any stderr line with the diagnostic template format? That would ensure that if there is no stdout, we always have something to share with users:
|
It's not actionable so I think its confusing to print it as a "Warning". Is there any reason not to suppress it always? Or to rewrite it as something like "Note: dart format does not read analysis file includes". |
This sounds like a good approach to me, I'll try it out. |
3 participants
Previously the
stdoutandstderrfrom the format execution weremerged and there was special handling for 1 specific warning message
followed by a standard 2 lines of boilerplate.
Now
stdoutanstderrare read separately.stderris ignored unlessthere is no output on
stdout. This filters any warnings that didn'tprevent formatting, including the old hardcoded warning as well as a new
one around resolving
analysis_options.yamland any future warningsthat may come up.
When there is no
stdoutcontent treat thestderras using thediagnostic format. Ignore any lines that don't match the error format.
I'm not currently aware of any output here that we wouldn't want to
hide, but if error output becomes important for the user to see we may
need to revisit this design.
systemlisttojob_startwhich allows separating theoutput streams.
callback for when the output has been read.